-
Notifications
You must be signed in to change notification settings - Fork 841
cleanup ProxyProtocol and prevent memory leak #12680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
include/iocore/net/ProxyProtocol.h
Outdated
| #pragma once | ||
|
|
||
| #include "tscore/ink_memory.h" | ||
| #include <filesystem> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What needs this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk why my editor things filesystem is needed in this file... I'll clean that up
| REQUIRE(tlv.has_value()); | ||
| CHECK(tlv.value() == "test"); | ||
| } | ||
| // Object destroyed here - ASAN will catch any memory leaks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat
This adds the rule of 5 functions to ProxyProtocol to allow safe copy construction and assignment.
It also avoids a memory leak of the use of ProxyProtocol in HttpTransaction::State where it wasn't getting destructed (HttpSM has a State and is class allocated without the flag to run the destructor).
#12676 will fix the memory leak in a different way, but the rule of 5 is still needed where ProxyProtocol is copied.